Feat: add `mithril-client` CLI command for UTxO-HD ledger state snapshot conversion #2518

dlachaume · 2025-05-23T16:18:02Z

Content

This PR includes...

Pre-submit checklist

Branch
- Tests are provided (if possible)
- Crates versions are updated (if relevant)
- CHANGELOG file is updated (if relevant)
- Commit sequence broadly makes sense
- Key commits have useful messages
PR
- All check jobs of the CI have succeeded
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested
Documentation
- Update README file (if relevant)
- Update documentation website (if relevant)
- Add dev blog post (if relevant)
- Add ADR blog post or Dev ADR entry (if relevant)
- No new TODOs introduced

Comments

Issue(s)

Closes #2492

…retrieve the `snapshot-converter` binary

github-actions · 2025-05-23T16:26:52Z

Test Results

3 files ± 0 77 suites ±0 14m 40s ⏱️ - 1m 14s
1 948 tests +17 1 948 ✅ +17 0 💤 ±0 0 ❌ ±0
3 329 runs +51 3 329 ✅ +51 0 💤 ±0 0 ❌ ±0

Results for commit cc19034. ± Comparison against base commit 623dbe8.

jpraynaud · 2025-05-27T09:55:54Z

mithril-client-cli/src/commands/tools/github_api_client/mod.rs

+/// Trait for interacting with the GitHub API to retrieve Cardano node release.
+#[cfg_attr(test, mockall::automock)]
+#[async_trait]
+pub trait GitHubApiClient {


If you need a trait for a GitHub API Client, you should do the following:

rename the module to github_release_retriever.rs

rename the trait to GitHubReleaserRetriever (and put it in an interface.rs sub-module?)

move the GitHubRelease in the module

rename the reqwest_github_api_client.rs sub-module to reqwest.rs

jpraynaud · 2025-05-27T09:58:47Z

mithril-client-cli/src/commands/tools/github_api_client/reqwest_github_api_client.rs

+        let url = Url::parse(&url)
+            .with_context(|| format!("Failed to parse URL for GitHub API: {}", url))?;
+
+        let response = self
+            .client
+            .get(url.clone())
+            .send()
+            .await
+            .with_context(|| format!("Failed to send request to GitHub API: {}", url))?;
+
+        let response = response.text().await?;
+        let release: GitHubRelease = serde_json::from_str(&response)
+            .with_context(|| format!("Failed to parse response from GitHub API: {:?}", response))?;


This code can be refactored in a function that retrieves the content from a URL and deserialize it into a generic type.
This download function could even be part of the HttpDownloader trait (by renaming the existing download function to download_file).

jpraynaud · 2025-05-27T09:59:59Z

mithril-client-cli/src/commands/tools/github_release.rs

+}
+
+impl GitHubRelease {
+    pub fn is_prerelease(&self) -> bool {


This function is probably interesting only if the visibility of the prerelease field is not public.

jpraynaud · 2025-05-27T10:01:57Z

mithril-client-cli/src/commands/tools/github_release.rs

+    fn dummy_linux_asset() -> GitHubAsset {
+        GitHubAsset {
+            name: format!("asset-name-{}.tar.gz", ASSET_PLATFORM_LINUX),
+            browser_download_url: "https://release-assets.com/linux".to_string(),
+        }
+    }


Maybe you can have only one function fn dummy_asset(os: &str) -> GitHubAsset here.

Alenar · 2025-05-27T10:15:39Z

mithril-client-cli/Cargo.toml

 fs2 = "0.4.3"
 futures = "0.3.31"
 human_bytes = { version = "0.4.3", features = ["fast"] }
 indicatif = { version = "0.17.11", features = ["tokio"] }
 mithril-cli-helper = { path = "../internal/mithril-cli-helper" }
 mithril-client = { path = "../mithril-client", features = ["fs", "unstable"] }
 mithril-doc = { path = "../internal/mithril-doc" }
+reqwest = { workspace = true }


The workspace only specify the minimal set of shared features for reqwest since we don't want to force them on downstream library users.
For our binaries we needs a bit more:

Suggested change

reqwest = { workspace = true }

reqwest = { workspace = true, features = [

"default",

"gzip",

"zstd",

"deflate",

"brotli"

] }

Alenar · 2025-05-27T12:42:50Z

mithril-client-cli/src/commands/tools/mod.rs

We should keep the commands module focused and move all utilities to the utils module (archive unpacker, github api client, http downloader).

Alenar · 2025-05-27T14:02:44Z

mithril-client-cli/src/commands/tools/archive_unpacker/zip_unpacker.rs

+        let root_file = temp_dir.join("root.txt");
+        assert!(root_file.exists());
+        let root_file_content = fs::read_to_string(root_file).unwrap();
+        assert_eq!(root_file_content, "root content");
+
+        let nested_file = temp_dir.join("nested/dir/nested-file.txt");
+        assert!(nested_file.exists());
+        let nested_file_content = fs::read_to_string(nested_file).unwrap();
+        assert_eq!(nested_file_content, "nested content");


Nitpick: You could use assert_dir_eq for the existence check.

Alenar · 2025-05-27T14:20:51Z

mithril-client-cli/src/commands/tools/archive_unpacker_selector.rs

Is there any value to return a ArchiveUnpacker instance to this api users ? It seems to exist only to enable testing.

It would refactor a bit this part:

Rename ArchiveUnpacker trait to ArchiveFormat with two methods: the actual unpack method and a support(&path) -> bool method

Rename ArchiveUnpackerSelector to ArchiveUnpacker:

with two methods: a public unpack(&path) -> Result<()> method and a private select_unpacker method

It would contains a list of ArchiveFormat impl and a Default impl that would manually add the two known format to this list.

select_unpacker would iterate over its format list and return the first unpacker which support the given archive

unpack would simply call select_unpacker then do the unpacking.

Alenar · 2025-05-27T14:21:55Z

mithril-client-cli/src/commands/tools/archive_unpacker/tar_gz_unpacker.rs

+        let root_file = temp_dir.join("root.txt");
+        assert!(root_file.exists());
+        let root_file_content = fs::read_to_string(&root_file).unwrap();
+        assert_eq!(root_file_content, "root content");
+
+        let nested_file = temp_dir.join("nested/dir/nested-file.txt");
+        assert!(nested_file.exists());
+        let nested_file_content = fs::read_to_string(&nested_file).unwrap();
+        assert_eq!(nested_file_content, "nested content");


Nitpick: You could use assert_dir_eq for the existence check.

Alenar · 2025-05-27T14:33:19Z

mithril-client-cli/src/commands/tools/snapshot_converter.rs

+const LEDGER_DIR: &str = "ledger";
+
+#[derive(Debug, Clone, ValueEnum)]
+enum UTxOHDFlavor {


Do we want to support lower case (and maybe only lower case) here ?

Alenar · 2025-05-27T14:34:41Z

mithril-client-cli/src/commands/tools/snapshot_converter.rs

+    /// Main command execution
+    pub async fn execute(&self) -> MithrilResult<()> {
+        let distribution_temp_dir = self.db_directory.join(CARDANO_DISTRIBUTION_TEMP_DIR);
+        std::fs::create_dir(distribution_temp_dir.clone()).with_context(|| {


Most fs methods takes AsRef<Path> as input which accept both moving the provided value or passing it by reference.

Suggested change

std::fs::create_dir(distribution_temp_dir.clone()).with_context(|| {

std::fs::create_dir(&distribution_temp_dir).with_context(|| {

dlachaume added 4 commits May 23, 2025 17:39

feat(client-cli): scaffold new command to convert ledger state snapshots

5cc0614

feat(client-cli): implement download of Cardano node distribution to …

3d9aadf

…retrieve the `snapshot-converter` binary

feat(client-cli): implement unpack of Cardano node distribution

063d2c3

[WIP]feat(client-cli): execute the snapshot-converter binary

cc19034

dlachaume self-assigned this May 23, 2025

dlachaume temporarily deployed to testing-preview May 23, 2025 16:27 — with GitHub Actions Inactive

jpraynaud reviewed May 27, 2025

View reviewed changes

Alenar reviewed May 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feat: add `mithril-client` CLI command for UTxO-HD ledger state snapshot conversion #2518

Feat: add `mithril-client` CLI command for UTxO-HD ledger state snapshot conversion #2518

dlachaume commented May 23, 2025

Uh oh!

github-actions bot commented May 23, 2025

Uh oh!

jpraynaud May 27, 2025

Uh oh!

jpraynaud May 27, 2025

Uh oh!

jpraynaud May 27, 2025

Uh oh!

jpraynaud May 27, 2025

Uh oh!

Alenar May 27, 2025

Uh oh!

Alenar May 27, 2025

Uh oh!

Alenar May 27, 2025

Uh oh!

Alenar May 27, 2025

Uh oh!

Alenar May 27, 2025

Uh oh!

Alenar May 27, 2025

Uh oh!

Alenar May 27, 2025

Uh oh!

Uh oh!

-reqwest = { workspace = true }
+reqwest = { workspace = true, features = [
+    "default",
+    "gzip",
+    "zstd",
+    "deflate",
+    "brotli"
+] }

	std::fs::create_dir(distribution_temp_dir.clone()).with_context(\|\| {
	std::fs::create_dir(&distribution_temp_dir).with_context(\|\| {

Feat: add mithril-client CLI command for UTxO-HD ledger state snapshot conversion #2518

Are you sure you want to change the base?

Feat: add mithril-client CLI command for UTxO-HD ledger state snapshot conversion #2518

Conversation

dlachaume commented May 23, 2025

Content

Pre-submit checklist

Comments

Issue(s)

Uh oh!

github-actions bot commented May 23, 2025

Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Feat: add `mithril-client` CLI command for UTxO-HD ledger state snapshot conversion #2518

Feat: add `mithril-client` CLI command for UTxO-HD ledger state snapshot conversion #2518